Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Ruby implementation creates the explicit coercion
#to_h
method and the implicit coercion#to_hash
method on Message and Map. Ruby treats objects that respond to implicit coercion methods differently. I do not think Protobuf intends that Message and Map are to be treated implicitly as hashes. In fact, I think this should actually be considered a bug.For example, here are how the explicit and implicit methods such as
#to_h
and#to_hash
are used:Similarily, Protobuf Messages and Maps are not Ruby Hashes. They can generate hash representations, but they are not themselves hashes. They should not be used in place of hashes.
What does the presence of the
#to_hash
method mean? It means Ruby will treat these objects as if they were hashes. Consider the following method:You would expect you could pass two Protobuf objects to the method, but you cannot.
This is because Ruby sees that the
blue
object has defined#to_hash
and treats it as a hash, and tries to match the optional named argument instead of the positional object.Because of this, and because there is no documentation that states that this method is intended to be part of the public API, I think these methods can and should be removed.